Skip to content

remove get_value_box since it is redundant with libindex.get_value_at #18371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 20, 2017
Merged

remove get_value_box since it is redundant with libindex.get_value_at #18371

merged 2 commits into from
Nov 20, 2017

Conversation

@jbrockmendel
Copy link
Member

tslib.get_value_box is effectively identical to index.get_value_at (figuring this out requires looking at util.get_value_at). Since the only use of get_value_box is in core.index.base, just get rid of it and use libindex.get_value_at there instead.

Fixup type declarations in util.get_value_at.

@jreback
Copy link
Contributor

jreback commented Nov 19, 2017 •
edited
Loading

pls asv indexing (shouldn't be an issue, but just check).

@jreback jreback added Clean Indexing Related to indexing on series/frames, not to indexes themselves Datetime Datetime data dtype labels Nov 19, 2017
@jbrockmendel
Copy link
Member Author

Turns out get_value_box is necessary because it raises IndexError in cases where the other version ignores it. So now the change is just to move get_value_box from libts to libindex.

@codecov
Copy link

codecov bot commented Nov 20, 2017 •
edited
Loading

Codecov Report

Merging #18371 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18371      +/-   ##
==========================================
- Coverage   91.38%   91.36%   -0.03%     
==========================================
  Files         164      164              
  Lines       49790    49799       +9     
==========================================
- Hits        45501    45499       -2     
- Misses       4289     4300      +11
Flag Coverage Δ
#multiple 89.16% <100%> (-0.01%) ⬇️
#single 39.55% <100%> (-0.04%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/base.py 96.42% <100%> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/indexes/interval.py 92.52% <0%> (-0.34%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️
pandas/core/internals.py 94.54% <0%> (ø) ⬆️
pandas/tseries/offsets.py 96.92% <0%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b00e62c...38e29d1. Read the comment docs.

jreback
jreback reviewed Nov 20, 2017
View reviewed changes
pandas/_libs/index.pyx


cpdef object get_value_box(ndarray arr, object loc):
cdef:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you can add a doc string at some point

@jreback jreback added this to the 0.22.0 milestone Nov 20, 2017
@jreback jreback merged commit 7aa2737 into pandas-dev:master Nov 20, 2017
@jreback
Copy link
Contributor

jreback commented Nov 20, 2017

thanks!

@jorisvandenbossche
Copy link
Member

@jbrockmendel a general comment: can you use the 'CLN: ' tag in PR titles / commits for PRs that are purely cleaning things up? (so no bug, enhancement or performance related aspect)

@jbrockmendel jbrockmendel deleted the get_value_box branch December 8, 2017 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jreback jreback jreback left review comments

Assignees

No one assigned

Labels

Clean Datetime Datetime data dtype Indexing Related to indexing on series/frames, not to indexes themselves

Projects

None yet

Milestone

0.23.0

Development

Successfully merging this pull request may close these issues.

get_value_box duplication

3 participants

@jbrockmendel @jreback @jorisvandenbossche